chore(hadoop): switch to hadoop shaded client#2767
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2767 +/- ##
============================================
+ Coverage 51.10% 51.75% +0.64%
- Complexity 3360 3978 +618
============================================
Files 534 600 +66
Lines 26055 33228 +7173
Branches 2442 3142 +700
============================================
+ Hits 13316 17197 +3881
- Misses 11880 14917 +3037
- Partials 859 1114 +255 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
wForget
changed the title
| <artifactId>commons-io</artifactId> | ||
| <version>${commons-io.version}</version> | ||
| <!-- commonis-io introduced by hadoop-client, so mark it as provided --> | ||
| <scope>provided</scope> |
There was a problem hiding this comment.
Pinning commons-io to provided in dependencyManagement breaks the standalone services (server/coordinator/storage). Their main code uses commons-io (FileUtils in LocalStorageChecker, IOUtils in coordinator), but the new declarations carry no scope and inherit provided. Since copy-dependencies only collects runtime scope, the -Phadoop-dependencies-included distribution no longer bundles commons-io. That distribution is meant to run without HADOOP_HOME, so the server throws NoClassDefFoundError: org/apache/commons/io/FileUtils at startup. CI misses it because the test classpath still has the provided jar. Declaring commons-io as <scope>${hadoop.scope}</scope> in those three modules aligns it with hadoop: provided by default, compile (bundled) under -Phadoop-dependencies-included.
There was a problem hiding this comment.
Thanks, added <scope>${hadoop.scope}</scope>
| HADOOP-17324: Hadoop 3.2.x hadoop-client-minicluster does not include the shaded | ||
| Bouncy Castle classes required by the Kerberos MiniDFS tests. | ||
| --> | ||
| <hadoop-minicluster.artifactId>hadoop-minicluster</hadoop-minicluster.artifactId> |
There was a problem hiding this comment.
The PR description says "Hadoop 3.2 and Spark 4 profiles use hadoop-client-minicluster", which contradicts the code. The hadoop3.2 profile actually pins back to the non-shaded hadoop-minicluster, with a HADOOP-17324 comment: the 3.2.x shaded jar lacks the BouncyCastle needed by the Kerberos MiniDFS tests. The code is correct; just fix the description, e.g. "only the Spark 4 profile switches to hadoop-client-minicluster; hadoop3.2 keeps hadoop-minicluster because the shaded jar lacks BouncyCastle".
| <hadoop.version>3.4.1</hadoop.version> | ||
| <hadoop-client-api.artifactId>hadoop-client-api</hadoop-client-api.artifactId> | ||
| <hadoop-client-runtime.artifactId>hadoop-client-runtime</hadoop-client-runtime.artifactId> | ||
| <hadoop-minicluster.artifactId>hadoop-client-minicluster</hadoop-minicluster.artifactId> |
There was a problem hiding this comment.
After spark4 switches to hadoop-client-minicluster, that shaded jar has no BouncyCastle, and spark4 (unlike hadoop3.2) adds no bcprov. It passes today only because -Duniffle.test.skip.kerberos=true skips all Kerberos tests. The @DisabledIfSystemProperty disabledReason only says "MiniKdc is not compatible with JDK 17+", which hides the second reason: spark4 also lacks BouncyCastle. If someone enables these tests under spark4 or drops the flag, KerberizedHadoop will fail the HADOOP-17324 way. Noting this dependency in the spark4 profile or that comment would save the next person the trip.
There was a problem hiding this comment.
Thansk, I added a comment to explain these two points.
| <dependency> | ||
| <groupId>commons-io</groupId> | ||
| <artifactId>commons-io</artifactId> | ||
| <scope>test</scope> |
There was a problem hiding this comment.
The test-scoped commons-io added to the client module isn't referenced anywhere under client/src (neither main nor test). Is it covering a transitive test dependency after the hadoop artifact switch, or just added along the way? Drop it if it's not needed; add a one-line comment if it is.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
The test-scoped commons-io added to the
clientmodule isn't referenced anywhere underclient/src(neither main nor test). Is it covering a transitive test dependency after the hadoop artifact switch, or just added along the way? Drop it if it's not needed; add a one-line comment if it is.
Yes, this is needed for a transitive test path after the Hadoop artifact switch. ShuffleReadClientImplTest exercises storage handlers from shuffle-storage, whose implementations use FileUtils/IOUtils. Since commons-io in shuffle-storage follows ${hadoop.scope}, it may resolve to provided and is therefore not guaranteed to reach the client test classpath. The explicit test dependency makes it available only to client tests. I’ve added a one-line comment to clarify this.
|
Are you also working on adding support for Spark 4.2? If so, I’ll hold off on my changes, haha. |
I only made an initial attempt and ran into Jersey 3 compatibility issues. If you also have an implementation, please feel free to submit a PR. |
3 participants
What changes were proposed in this pull request?
Use Hadoop's shaded client artifacts for the Spark 4 profiles while preserving the existing artifacts for older Hadoop profiles.
This PR introduces shared properties for the Hadoop client API, runtime, and minicluster artifact IDs, and updates the affected modules to use these properties instead of hard-coded artifact IDs:
hadoop-common,hadoop-client, andhadoop-minicluster.hadoop-miniclusterbecausehadoop-client-miniclusterin Hadoop 3.2.x does not contain the shaded Bouncy Castle classes required by the Kerberos MiniDFS tests (HADOOP-17324).hadoop-client-api,hadoop-client-runtime, andhadoop-client-minicluster.The PR also adds explicit
commons-iodependencies where they are no longer provided transitively and updatesMapUtilsto use Commons Collections 4.Why are the changes needed?
While investigating a Spark 4.2 upgrade, a test classpath conflict was found between Jersey 3 used by Spark and Jersey 2 brought in through the non-shaded Hadoop artifacts.
Using the shaded Hadoop client artifacts for Spark 4 avoids this dependency conflict. Property-based artifact selection keeps the existing Hadoop 2.8 and Hadoop 3.2 builds compatible while allowing Spark 4 to use the cleaner shaded dependency set.
This PR only adjusts Hadoop dependency selection; it does not add Spark 4.2 support.
How was this patch tested?
existing unit tests